Skip to content

CMake: Support for XML documentation #1682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 13, 2025
Merged

CMake: Support for XML documentation #1682

merged 1 commit into from
Jan 13, 2025

Conversation

enetheru
Copy link
Collaborator

@enetheru enetheru commented Jan 12, 2025 •
edited
Loading

This PR extends #1499 - [DRAFT] Add CMake support for XML documentation
which follows #1374 - Allow submitting documentation to the Godot editor

This is also built upon #1670 - CMake: Support for using build_profile.json

This PR adds the equivalent functionality to the CMake build scripts. A summary below.

The SCons implementation adds a BUILDER to the env in tools/godotcpp.py:513

    # Builders
    env.Append(
        BUILDERS={
            "GodotCPPBindings": Builder(action=Action(scons_generate_bindings, "$GENCOMSTR"), emitter=scons_emit_files),
            "GodotCPPDocData": Builder(action=scons_generate_doc_source),
        }
    )

Which is then available for consumers to use, with the test example showing how at test/SConstruct:18

if env["target"] in ["editor", "template_debug"]:
    doc_data = env.GodotCPPDocData("src/gen/doc_data.gen.cpp", source=Glob("doc_classes/*.xml"))
    sources.append(doc_data)

This CMake implementationa mirrors this with a function in cmake/python_callouts.cmake:109

function( generate_doc_source OUTPUT_PATH XML_SOURCES )
- - - <snip> - - -

which calls the python function of the same name from doc_source_generator.py to create the cpp file at OUTPUT_PATH which can then be added to the sources list of the target. The test extension shows this starting in test/CMakeLists.txt:10

# Generate Doc Data
file( GLOB_RECURSE DOC_XML
    LIST_DIRECTORIES NO
    CONFIGURE_DEPENDS
    ${CMAKE_CURRENT_SOURCE_DIR}/doc_classes/*.xml )

set( DOC_DATA_SOURCE "${CMAKE_CURRENT_BINARY_DIR}/src/gen/doc_data.gen.cpp" )
generate_doc_source( "${DOC_DATA_SOURCE}" "${DOC_XML}" )

- - - <snip> - - - 

    # conditionally add doc data to compile output
    if( TARGET_ALIAS MATCHES "editor|template_debug" )
        target_sources( ${TARGET_NAME} PRIVATE "${DOC_DATA_SOURCE}" )
    endif(   )

@enetheru enetheru mentioned this pull request Jan 12, 2025
Closed
@enetheru
Copy link
Collaborator Author

@dsnopek should I add the main function in the doc_source_generator.py to match the latest build_profile.py ?

@dsnopek
Copy link
Collaborator

dsnopek commented Jan 12, 2025 •
edited
Loading

Thanks! You can squash my commit in your commit, and I can close mine as superseded by yours.

should I add the main function in the doc_source_generator.py to match the latest build_profile.py ?

I think I'd actually prefer that we keep this code in binding_generator.py rather than creating a new doc_source_generator.py.

In my mind, build_profile.py is the exception, because it could be used by other bindings and we even discussed moving that functionality into Godot itself, so it makes sense for that to be a separate script with a main function.

However, the documentation stuff is squarely a part of godot-cpp, and I'd like to keep the number of Python scripts at the top-level to a minimum.

UPDATE: I changed my mind on this bit - see comment below.

@dsnopek
Copy link
Collaborator

dsnopek commented Jan 12, 2025

I think I'd actually prefer that we keep this code in binding_generator.py rather than creating a new doc_source_generator.py.

In my mind, build_profile.py is the exception, because it could be used by other bindings and we even discussed moving that functionality into Godot itself, so it makes sense for that to be a separate script with a main function.

However, the documentation stuff is squarely a part of godot-cpp, and I'd like to keep the number of Python scripts at the top-level to a minimum.

Er, nevermind, thinking about this a little more, I think it's fine as a separate script. I don't want to end up with too many scripts at the top-level, but this is distinct enough from everything else binding_generator.py is doing, let's just leave it separate.

However, to answer the original questsion: no, I don't think doc_source_generator.py needs a main function. :-)

@dsnopek dsnopek added enhancement This is an enhancement on the current functionality cmake topic:buildsystem Related to the buildsystem or CI setup labels Jan 12, 2025
@dsnopek dsnopek added this to the 4.x milestone Jan 12, 2025
dsnopek
dsnopek reviewed Jan 12, 2025
View reviewed changes
Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried this with the test project and it worked great!

Skimming the code, that looks good to me too.

I think the only things this PR needs is:

  1. Squash my commit and your commit like I mentioned above (you can list me as Co-authored-by: in the commit if you want, but my contribution is pretty small, I don't mind if it's omitted)
  2. Rebase on master, since I just merged PR CMake: Support for using build_profile.json #1670

@enetheru @dsnopek
CMake: Support for XML documentation
8814ac5 
Add new function generate_doc_source to python_callouts.cmake
Update Test extension to use generate_doc_source function and include generated file in the build.

Cleanup:
Fix godotcpp.py imports after rebase
Pre-Commit hook sorted python imports in doc_source_generator.py
- replace ${CMAKE_CURRENT_SOURCE_DIR} with ${godot-cpp_SOURCE_DIR} when referencing current working directory and script locations when invoking python scripts.

Co-authored-by: David Snopek <dsnopek@gmail.com>
@enetheru enetheru marked this pull request as ready for review January 12, 2025 21:59
@enetheru enetheru requested a review from a team as a code owner January 12, 2025 21:59
@dsnopek dsnopek merged commit befe3ee into godotengine:master Jan 13, 2025
11 checks passed
@enetheru enetheru deleted the gdext-docs-cmake branch January 13, 2025 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dsnopek dsnopek dsnopek approved these changes

Assignees
No one assigned
Labels
cmake enhancement This is an enhancement on the current functionality topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Milestone
4.x
Development

Successfully merging this pull request may close these issues.
2 participants
@enetheru @dsnopek